feat(studio): motion-path geometry + commit helpers#1609
Conversation
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Review @ ec4218d2 — pure-additions substrate for the motion-path overlay (geometry, commit helpers, selection resolution, occlusion test). Reviewed standalone + against #1610's consumer call sites.
Looks good
- No tautology risk. Every new export has a non-test production consumer in #1610:
buildMotionPathGeometry(MotionPathOverlay.tsx:205),nearestPointOnPath(:433/:445),editableAnimationId(:336),selectorFor(:241/:260),commitNode(:420),commitAddWaypoint(:455),commitAddKeyframe(:466),commitRemoveWaypoint(:474),commitCreatePath(:279).isElementVisibleInPreviewis consumed by both MotionPathOverlay ANDuseDomEditOverlayRects.ts:151— strong cross-overlay parity, exactly the kind of shared visibility rule that prevents drift. - Single-axis tween regression is explicitly tested (
points: \"0,0 -260,0\"and\"0,0 0,500\"); good catch on the priorskip-if-missing-yfoot-gun. nearestPointOnPathhandles coincident nodes (len2 === 0 → t = 0); endpoint clamps tested; segment indexing tested.buildMotionPathGeometryreturnsnullcleanly on<2nodes, null input, opacity-only tweens. NaN-safe viaNumber.isFiniteaxis guard.editableAnimationId's!a.provenancegate keeps helper tweens read-only — matches the overlay's drag-disabled path.
Concerns
isElementVisibleInPreviewcouples to a one-way fade assumption (domEditOverlayGeometry.ts:38-45). The docstring says "this composition stacks scenes by z-index and fades them IN (never out)." TheeffectiveOpacity > 0.01cutoff treats anything mostly-opaque as an opaque cover and bails. If a future composition ever fades scenes OUT (or animates opacity through mid-values during a crossfade), an element under a 0.5-opacity cover will be classified as occluded → motion path + selection box both disappear during the crossfade. Worth either (a) a brief comment in the docstring pinning this to a runtime invariant, or (b) loosening the cutoff (e.g.> 0.85) so a mid-fade isn't read as a hard cover. Cite the assumption explicitly somewhere a future scene-author will see it.- Occlusion sample set is corners + center (
OCCLUSION_SAMPLE_POINTSat 20%/50%/80%). Donut-shaped elements (large bounding box with transparent middle) would be wrongly classified as visible when all five samples hit transparent content. Today's text/image scenes don't hit this, but worth a code comment noting the assumption — it's the kind of thing a future scene type (a frame/ring decoration) silently regresses.
Nits
nearestPointOnPathreturnstclamped past-end → for a point past the last node, you getsegIndex = lastSegment, t = 1. Callers in #1610 usenp.segIndex + 1for insertion, which would insert AFTER the last node. Reasonable, but the JSDoc comment doesn't say whether "past the end" is the intended insertion semantics. One line clarifying "t = 1on the last segment means insert at the end" would help. (nit)- The doc comment block header
\"ponytail: the arc is drawn as a polyline...\"on line 35 ofmotionPathGeometry.tsis inside thebuildMotionPathGeometryJSDoc butnearestPointOnPathis declared first below it. Cosmetic doc-comment ordering only. (nit)
What I didn't verify
- I didn't run the test suite locally; I verified test shapes by reading.
- I didn't trace every mutation type (
update-keyframe,add-motion-path-point, etc.) into the GSAP parser to confirm the payload shape is whatcommitMutationexpects — relied on PR body + the commit helpers' tests covering payload shape.
Stamp-routable from my side once the occlusion docstring caveat is acknowledged (either fix or accept-as-known-assumption). Geometry math + helpers are solid.
— Rames D Jusso
vanceingalls
left a comment
There was a problem hiding this comment.
Reviewed at ec4218d2. Concur with @james-russo-rames-d-jusso — solid foundation slice. Pure helpers with strong unit-test coverage and the right cross-overlay parity hook.
Verified at HEAD:
motionPathGeometry.test.ts coverage matrix is comprehensive and asserts against expected payloads, not just verify-no-throw:
- linear path + ordering/percentages preserved
- arc path with waypoint refs (kind === "arc", indices 0/1/2)
- opacity-only → null
- single-axis x-only (
"0,0 -260,0") and y-only ("0,0 0,500") — explicit regression cases - skip-keyframe-missing-axis (
excludes keyframes missing a coordinate without throwing) - <2 valid nodes / null input
- nearestPointOnPath: nearest-segment + t-fraction, t-clamp at endpoints, second-segment selection, past-segment endpoint clamp, <2 nodes → null
No stubs, no escape hatches.
Commit helper dispatch — verified handlers exist in routes/files.ts at HEAD:
commitNode(keyframe) →update-keyframe(files.ts:1033)commitNode(waypoint) →update-motion-path-point(:1082)commitAddWaypoint→add-motion-path-point(:1088)commitAddKeyframe→add-keyframe(:1020)commitRemoveWaypoint→remove-motion-path-point(:1094)commitCreatePath→add-motion-path(:1097)
All six route to real handlers in the recast switch. Note: routes/files.ts carries TWO switch blocks (executeGsapMutationRecast + executeGsapMutationAcorn), each with add-keyframe / update-keyframe cases — these are cutover branches gated by STUDIO_SDK_CUTOVER_ENABLED, not racing. Dispatch IS determinate. But the acorn switch is missing all four motion-path cases (see my #1555 review) — dark-launch breakage when the cutover flag flips.
Concur with @james-russo-rames-d-jusso:
isElementVisibleInPreviewcouples to a one-way-fade assumption atdomEditOverlayGeometry.ts:38-45. Worth either a docstring caveat or loosening theeffectiveOpacity > 0.01cutoff so mid-crossfade isn't read as a hard cover. The shared rule withuseDomEditOverlayRectsis the right call (path + selection box share one truth) — that's the strongest cross-overlay parity win in the PR. Just pin the assumption.- Donut-shaped occlusion samples: corners + center won't catch transparent middles. Comment naming the assumption suffices for now.
Net-new (NIT, soft cross-PR drift to #1610):
readRuntimeKeyframes (already on main) explicitly handles the multi-tween case by returning the tween under the playhead — its source comment names this production case. editableAnimationId in this PR picks the FIRST animation matching the predicate, playhead-blind. For an element with two position-keyframed tweens at disjoint time ranges, the geometry is drawn from playhead-tween but commitNode/commitAdd* will write via the first-match animationId — potentially the wrong tween. Single-tween case (dominant) is correct.
Likely fails loud server-side (id mismatch → structured error) rather than silent wrong-write, but the runtime helper's own comment documents this as a real production case. NIT not BLOCK; future fix would key the picker on the same time window the geometry came from. Surfaces at the commit boundary in #1610.
LGTM. No band-aid patterns. Tests assert against expected output, helpers are real-consumer-grounded.
Review by Via
…iant, donut limit, nearestPointOnPath t-semantics
57f4cd8 to
c66e82a
Compare
ec4218d to
395c6f7
Compare
|
Subsumed by #1605 (retargeted to main with the full stack) |
* chore(producer): shim __filename/__dirname in the CJS banner Bundled CJS deps like wawoff2 call __dirname; without the shim they throw "__dirname is not defined in ES module" at render time. Also ignore .zed/. * chore(producer): use a template literal for the CJS banner (review nit) * feat(core): add GSAP keyframe + motion-path source mutations Array-form keyframe removal in both the recast and acorn writers, plus update/add/remove-motion-path-point and add-motion-path. Exclude _auto and data from tween property-group classification. * fix(core): address #1554 review — data-exclusion test, split-fix doc, motion-path sentinel, parity blocks - Regression test for the `data` GSAP-key exclusion (parallel to _auto). - splitAnimationsInScript: documented that .fromTo()/.to() correctly stay out of the from-branch (only .from() reverts) and the <= boundary; added mid-flight straddle tests. - addMotionPathToScript failure path returns id: null (was empty-string sentinel); caller updated. - Parity blocks for addKeyframeToScript array-form + updateKeyframeInScript (mirroring removeKeyframeFromScript). Surfaced a latent acorn array-form partial-props merge bug — documented as it.skip with a ready assertion (acorn cutover follow-up). * feat(core): route motion-path mutations through studio-api + fix clip stamping Wire the new mutations into the file save route. Only authored clips suppress descendant stamping, so auto-stamped animated scenes can inline-expand. Hide in-flow timed clips with `display:none` only when they are LEAF clips (no nested timed clips). `display:none` on a container removes its whole subtree, hiding descendants that are still inside their own visibility window — e.g. an in-flow composition root whose effective window clamps to the timeline end would black out a child video that should still show (the hdr-hlg regression). Containers keep `visibility:hidden`, which a visible descendant can override; only leaves leave the flow, which is all the split-overlap case needs. * feat(core): strip legacy path-offset/rotation + drop obsolete studio lint rule A position or rotation add/set mutation makes the GSAP timeline the single source of truth for that channel, so any lingering --hf-studio-offset / --hf-studio-rotation CSS var must be cleared to avoid double-applying. stripStudioEditsFromTarget now clears both channels, and the add-strip fires for the position AND rotation property groups. Also removes the obsolete `gsap_studio_edit_blocked` lint rule: it warned that Studio cannot save drag/resize edits to elements in a registered timeline — the exact premise the single-source work inverts (the timeline is now the edit target). Removed the rule, its now-unused TIMELINE_REGISTRY_ASSIGN_PATTERN import, and its 5 tests. * fix(core): address #1555 review — complete hold-sync, invalidate clip cache, strip rotation channel - HOLD_SYNC_MUTATION_TYPES: add add-motion-path (load-bearing — addMotionPathToScript authors past t=0 → first-frame snap-to-(0,0) without the hold), update-meta, shift-positions, scale-positions, split-animations. (add stays out: flat tweens only, syncPositionHoldsBeforeKeyframes is a no-op for non-keyframed tweens.) - init.ts: timedClip in-flow/leaf WeakMaps now invalidate on clipTreeSignature change; visible/hidden branches both go through isTimedClipInFlow (was .get() by accident). - keyframesWriteRotation mirrors keyframesWritePosition so a rotation-only keyframe set strips the stale --hf-studio-rotation channel. * feat(studio): GSAP runtime read layer + shared helpers * fix(studio): address #1607 review — cold-parse vs fetch-error budgets, isZeroDurationSet, array-ease tests - useGsapAnimationFetchFallback: discriminate resolved/fetch-error/cold; only the cold (warm-but-zero) race gets the full ~600ms retry budget — a hard fetch error retries once. - Extract isZeroDurationSet (was !(duration>0) duplicated); rejects NaN, documents intent. - parsePercentageKeyframes: cite GSAP even-index spread; tests that a per-entry/interior ease is stripped without shifting the other keyframes' percentages. * feat(studio): GSAP drag/commit/bridge editing infra * fix(studio): address #1608 review — facade awaits commit, strict stale-parse guard, clearProps restore BLOCKER: useSafeGsapCommitMutation now RETURNS the (.catch-chained) commit promise and the commitMutation facade awaits it — so await session.commitMutation(...) resolves AFTER the server save, fixing both consumers (useEnableKeyframes + useGestureCommit's showToast/requestSeek/idle, which were firing before the save landed). SafeGsapCommitMutation return type widened void→Promise<void> (fire-and-forget consumers ignore it). - stale-parse guard uses hasNonHoldTweenForElement (a leftover hold set no longer counts as live). - commitFlatViaKeyframes snapshots dragged gsap values before clearProps + restores after seek, so a failed commit leaves the dropped pose, not a cleared element. * feat(studio): motion-path geometry + commit helpers * docs(studio): address #1609 review — document occlusion fade-in invariant, donut limit, nearestPointOnPath t-semantics * feat(studio): on-canvas motion-path overlay * fix(studio): address #1610 review — scope dblclick to pan-surface, kind-aware geometry guard, gate createMode, screen-space drag threshold * feat(studio): keyframes flag, gesture recording + timeline/selection refinements * fix(studio): address #1611 review — fetch-first keyframe path, gated hydration, dev-gated debug + gesture warn, per-group gesture tweens - useEnableKeyframes: parse current source first (null-vs-[] distinction) so a delete-all's empty parse isn't overridden by a stale selectedGsapAnimations cache. - useStudioUrlState: freeze the hydration effect's time dep once hydrated (was re-running every tick). - useGestureRecording: dev-gated console.warn when the live-preview runtime throws (was silent). - playerStore: gate window.__playerStore behind dev (guarded import.meta.env.DEV). - useGestureCommit: partition recorded keyframes by property group → one add-with-keyframes per group, so a mixed gesture no longer yields an untagged legacy tween. * feat(studio): single-source manual offset + rotation via the GSAP timeline Dragging or rotating an element writes into the GSAP timeline (the single source of truth) instead of a parallel --hf-studio-offset / --hf-studio-rotation CSS var: static elements commit a tl.set (idempotent on re-edit), tweened elements edit keyframes, and the live preview moves via gsap.set so what you see equals what is written and renders. Removes the dual-channel CSS-var/transform reconciliation behind the fling / disappear / runaway / double-stack / wrong-start bug class — for BOTH position and rotation (gesture base read from the gsap transform, gsap.set live preview, tl.set/ keyframe commit, dropped the handleDom*Commit CSS fallbacks). Subcompositions edit the same single-source way, which surfaced and fixes: - resolve a subcomp element's source file via the composition-id map (the runtime drops the source linkage when inlining the subcomposition); - a selected element's selection box AND motion path use basic visibility, not the occlusion heuristic (a backgroundless opacity-1 scene above it is not an opaque cover); - soft reload rebuilds ONLY the committed composition's timeline, leaving other compositions' timelines intact (no cross-composition revert); - read keyframes from the element's OWN composition timeline (scan all timelines, not the first unstable key); - delete-all uses a soft reload too, so editing no longer hard-reloads the iframe. * fix(studio): address #1567 review — drop drag-intercept flag, harden softReload onerror, tighten runtime ladder, per-group gestures - DROP STUDIO_GSAP_DRAG_INTERCEPT_ENABLED: single-source GSAP intercept is the only position/rotation channel; the false branch silently killed drag+rotate (and let GSAP elements into the keyframe-corrupting CSS path). Removed flag + dead branch + env def + tests. - gsapSoftReload: plugin onerror no longer fakes success — signals onAsyncFailure so the caller full-reloads; honors __hfMotionPathPluginLoading so a concurrent reload can't queue a dup script. - gsapDragCommit: resolveDragRuntime narrows the as-any ladder; a mid-seek throw logs + drops partial reads (no phantom identity) and re-applies the drag override in finally. - MotionPathOverlay: park-timer cleanup keyed on animId change. - useGestureCommit: partitionKeyframesByGroup wraps the add-with-keyframes sites (per #1611 review). * feat(studio): patchRuntimeTweenInPlace — update a tween's values in place Defensive runtime helper: locate the element's tween in window.__timelines via the shared resolveRuntimeTween scan, update its set/keyframe vars, invalidate, and re-seek the playhead — without re-running the whole composition. Returns false (caller falls back to soft reload) for any shape it can't safely patch (no tween, dynamic/computed keyframes, motionPath arc, channel mismatch, or any error). Foundation for instant, flicker-free manual edits. * fix(studio): address #1612 review — channel-aware set resolution + decline dynamic-expression patches - resolveRuntimeTween gains an optional channels[] hint; for kind:set it prefers the set whose vars carry one of the patched channels and never returns a disjoint-only set (e.g. won't write {x,y} into a co-located {rotation} set). patchRuntimeTweenInPlace derives channels from the props. - patchSet declines (returns false → soft reload) when overwriting a string/dynamic vars[ch], instead of silently dropping the computed expression. * feat(studio): instantPatch fast path in runCommit A commit carrying an instantPatch option tries patchRuntimeTweenInPlace first; on success the preview updates in place with NO reload (instant), on false it falls back to the existing soft reload. Extracts the preview-sync tail into a testable applyPreviewSync helper. No behavior change when instantPatch is absent. * feat(studio): route static position/rotation set drags through instantPatch Static-element position and rotation set commits now attach instantPatch{selector, change:{kind:set}} so the drag updates in place with no reload. Structural ops (new tween add, delete-all, convert/split/materialize) and keyframe edits deliberately omit it and keep the soft reload — keyframe instant-patch needs object-form keyframe support in patchRuntimeTweenInPlace (deferred). * fix(studio): address #1613 review — derive instantPatch from the mutation, patch both coalesced commits, wire onAsyncFailure - commitStaticGsapPosition/Rotation derive instantPatch.change.props from the actual update-property mutation(s) sent (one source of truth → findUnsafeMutationValues-validated values flow into the patch; can't drift). - Coalesced x/y: the intermediate x commit also carries instantPatch{x}, the y commit {x,y}, so a second-POST failure still leaves the preview patched for what persisted. - applyPreviewSync passes reloadPreview as onAsyncFailure (plugin-CDN load error → full reload); per U4 the synchronous false still does NOT escalate. - (channel disambiguation from #1612 verified end-to-end: {x,y}→position set, {rotation}→rotation set.) * feat(studio): no full iframe remount for soft-reloadable edits A softReload edit (and the SDK single-script refresh) no longer escalates to a full reloadPreview() iframe remount when applySoftReload returns false — the live gsap.set already shows the value, and a remount is the worst flash + re-inlines subcomps (reverting their keyframes). verifyTimelinesPopulated now checks the expected target keys the re-run registers, so a correct scoped re-run doesn't spuriously report empty. Full reload stays only for the structural (no-softReload) and ambiguous-script paths. * feat(studio): pre-load MotionPathPlugin so motion-path edits don't async-flash ensureMotionPathPluginLoaded() runs once at the preview iframe-load seam (NLELayout onIframeLoad), eagerly loading + registering MotionPathPlugin without killing the timeline. So when a user adds a motion path to a composition that didn't originally use one, the soft reload runs synchronously instead of taking the kill-then-await-CDN async path (the flash). Idempotent + defensive; the existing async fallback stays for genuine cold-start/CDN-failure. * fix(studio): don't re-save + reload when source editor syncs externally The SourceEditor's CodeMirror update listener fired onChange on ANY docChanged — including the programmatic dispatch that syncs external content (e.g. a manual-edit commit writing the source back into the open editor). That made the editor re-save the file and bump refreshKey, fully reloading the preview iframe on every drag/keyframe edit — defeating the in-place instant patch and causing the flash. Annotate the programmatic sync (ExternalSync) and skip onChange for it, so only real keystrokes save. * fix(core): inject MotionPathPlugin into preview when a composition uses motionPath A studio-created motion path writes a gsap motionPath tween into the single-source timeline, but the preview HTML only loaded gsap core — so the first render threw "Invalid property motionPath ... Missing plugin?". Detect motionPath usage and inject MotionPathPlugin right after the composition's gsap script, version-matched to it. * fix(studio): dedup __hfMotionPathPluginLoading type decl (restack artifact) * fix(studio): address #1605 review — distinguish soft-reload failure modes + observability, SourceEditor focus guard BLOCKER: applySoftReload now returns SoftReloadResult ('applied' | 'verify-failed' | 'cannot-soft-reload') instead of a bare bool. applyPreviewSync + sdkRefresh escalate to a full reloadPreview() on the PERMANENT 'cannot-soft-reload' (no gsap/rebind hook/scopable key/script, or sync re-run threw) — fixing the silent-stale-preview U4 dropped — but still suppress the TRANSIENT 'verify-failed' (live gsap.set is correct). Telemetry: gsap_soft_reload_outcome (origin/result/escalated) + gsap_instant_patch_fallback, so the U4 invariant is enforced, not asserted. - SourceEditor: skip the programmatic external-sync replace while the editor is focused, so an in-flight commit doesn't clobber the user's uncommitted keystrokes (ExternalSync kept for unfocused). - Verified ensureMotionPathPluginLoaded already guards __hfMotionPathPluginLoading (no double-append). * fix(core): align __clipTree and __clipManifest ids via stableClipId Timeline inline expansion was dead for nested children inside index.html: the tree keyed id-less elements by a synthetic __clip-N while the manifest keyed them null, so parent<->child never joined. Both now resolve identity through stableClipId (id || data-hf-id), which every generated element has. * fix(core): strip baked runtime + tag comp root in preview assembly Comps that ship a baked inline runtime were double-loaded (preview injects its own) and the baked copy failed to parse inline (Unexpected token '<'). Strip it in buildSubCompositionHtml + the disk-fallback preview path. Also tag the comp root with data-composition-file so the studio resolves a comp's top-level elements to the right source file instead of defaulting to index.html (which made the GSAP panel parse the wrong, multi-timeline file). * feat(studio): set motion-path destination from a toolbar toggle Replaces the double-click-on-canvas UX (which painted text over the preview) with a 'Set motion destination' toggle next to Snap/Grid, shown only when the selected element can take a path. While armed, one canvas press places the destination. Also removes the dead TimelinePropertyRows component. * fix(studio): center timeline keyframe diamonds on their percentage Dropped clampDiamondLeft, which forced boundary keyframes fully inside the clip so a 0% diamond sat half a diamond right of the 0% point. Each diamond's midpoint now sits exactly on its % (the clip is overflow-visible). * fix(studio): resize static elements via tl.set, not a single-stop keyframes tween Resizing an element with no size animation wrote keyframes:{ <playhead%>: {width,height} } — one mid-point stop GSAP can't interpolate, so it rendered NaN/0 dimensions at every other frame and the element vanished (worst off 0%). Added commitStaticGsapSize (mirrors commitStaticGsapPosition): a static resize now writes tl.set({width,height}), held at all frames; re-resizing updates it in place. * fix(studio): negative-cache failed media probes Only successful probes were cached, so CORS/404 cross-origin media was re-probed every rAF-driven timeline re-derive, flooding the console. Remember failed URLs and skip them. * fix(studio): type window.setTimeout handle as number ReturnType<typeof window.setTimeout> infers NodeJS.Timeout when @types/node is present and clashes with the DOM number the call returns. Type it number. * fix(studio): drag/resize disappearance, stale-ID duplicates, soft-reload clearProps - Fix soft-reload clearProps destroying element inline styles — save cssText, clear, restore, strip only transform - Fix resize no-op on re-resize: delete+add instead of two update-property - Route set tweens through static resize path (convertToKeyframes skips sets) - Re-fetch animation ID before drag commit to prevent stale-ID duplicates - Guard editDebugLog for Node test environments - Fix NLELayout setState-during-render (move reset to useEffect) - Stop SnapToolbar pointer events propagating to canvas deselect handler - Enable click-to-add waypoints on cubic motion paths - Add whole-path drag offset (Alt+drag shifts all keyframes together) - Add Canvas shortcuts section to ShortcutsPanel - Extract useMotionPathData + commitGsapPositionFromDrag (filesize compliance) - Delete dead code (getElementDepth, isElementVisibleInPreview, unused exports)

Summary
This PR adds the pure, testable core of the on-canvas motion-path editor: geometry construction, gesture-to-source-mutation commit helpers, selection resolution, and a preview-occlusion visibility test. Everything here is framework-free (no React, no DOM coupling beyond the one occlusion helper), so the logic that turns a live tween into a draggable polyline — and turns a drag into a GSAP source edit — is unit-tested in isolation. The overlay component itself lands later in the stack; this is the substrate.
Net change: 6 files, +537 lines (4 source modules, 2 test files). No existing code is modified except one additive export in
domEditOverlayGeometry.ts.What's changed
motionPathGeometry.ts(new)Converts a live tween from
readRuntimeKeyframes(ReadTween) into renderable overlay geometry, in composition space.MotionNodeRef— discriminated union:{ type: "keyframe"; pct }(an x/y position keyframe at a tween-relative percentage) or{ type: "waypoint"; index }(amotionPathanchor at a source index).MotionPathNode/MotionPathGeometry— a node is{ x, y, ref }; the geometry is{ kind: "linear" | "arc", points, nodes }wherepointsis an SVG polyline string.buildMotionPathGeometry(read)— returns geometry ornull.kindis"arc"when the tween carries anarcPath, else"linear". Arc keyframes are treated as path waypoints (index-aligned with source); linear nodes carry their tween-relative percentage. Single-axis tweens are handled explicitly (detectstweenHasX/tweenHasY, defaults the missing axis to 0) so an x-only or y-only tween still draws a straight path. A keyframe that omits an axis the tween does animate is skipped. Returnsnullfor no positional motion or fewer than two valid nodes. Arcs are drawn as polylines through waypoints (dense curve sampling is a deliberate later refinement).nearestPointOnPath(px, py, nodes)— projects a point onto the closest polyline segment, returning{ x, y, segIndex, t, dist }. Drives the ghost "add" node and where a new node lands. Returnsnullfor fewer than two nodes.motionPathCommit.ts(new)Maps a finished gesture to a GSAP source mutation through the selection-bound
commitfacade (which owns soft reload, undo snapshot, save-failure feedback). All helpers passsoftReload: true+ a human-readable undolabel.commitNode→update-keyframe(keyframe) orupdate-motion-path-point(waypoint).commitAddWaypoint→add-motion-path-point;commitAddKeyframe→add-keyframe(tween-relative %);commitRemoveWaypoint→remove-motion-path-point;commitCreatePath→add-motion-path(defaultNEW_PATH_DURATION1.5s).motionPathSelection.ts(new)Selection/animation resolution, split out to avoid a circular import between the overlay and its diagnostics.
selectorFor(sel)—#${CSS.escape(id)}preferred, falls back tosel.selector.editableAnimationId(animations, kind)— picks the one animation editable on-canvas: literal, statically resolved, matching the rendered geometrykind.okrequires!hasUnresolvedKeyframes && !hasUnresolvedSelector && !provenance. Returnsnullfor dynamic / helper-provenance tweens (read-only).domEditOverlayGeometry.ts(modified — additive)isElementVisibleInPreview(el)(new export) — extendsisElementVisibleForOverlaywith an occlusion hit-test. The composition stacks scenes by z-index and fades them in, so an earlier element can stay opacity 1 yet be covered. Walks the painted stack viaelementsFromPointat five sample points; a point "sees" the element if reached before any unrelated effectively-opaque element (transparent covers skipped). HelpereffectiveOpacity(el, win)computes cumulative ancestor opacity. If every in-viewport sample is blocked it's hidden; if nothing was testable it is not hidden on that basis.Why
The overlay must reliably (1) draw the path a tween traces and (2) translate a drag into a precise source edit — both easy to get subtly wrong (single-axis tweens drawing no path, arc waypoint indices drifting, an "add node" on the wrong segment, a node committing the wrong mutation). Pure modules let us pin that down with unit tests before any pixels/React state, keep the overlay thin, and break a circular import. The occlusion test exists because the stacked-scene fade-in makes naive opacity checks report covered elements as visible.
Testing
Two new Vitest suites:
motionPathGeometry.test.ts—buildMotionPathGeometry: linear path with keyframe refs; order/percentages preserved; arc path with waypoint refs;nullfor opacity-only; single-axis x-only ("0,0 -260,0") and y-only ("0,0 0,500") regression cases; skips keyframes missing a coord;nullfor <2 nodes / null input.nearestPointOnPath: nearest-segment projection,tat endpoints, second-segment selection, endpoint clamp,nullfor <2 nodes.motionPathCommit.test.ts—editableAnimationId(arc, position-keyframe,nullfor dynamic + helper-provenance + no-match);commitNoderoutes keyframe→update-keyframe, waypoint→update-motion-path-point(bothsoftReload: true); add/remove waypoint;commitAddKeyframe;commitCreatePath(1.5s default). All commit assertions verify both payload andsoftReload: true.Stack
Part of the GSAP keyframe/motion-path stack:
#1553 → #1554 → #1555 → #1607 → #1608 → #1609 → #1610 → #1611 → #1567 → #1612 → #1613 → #1605. This is #1609, on top of #1608. Builds independently; the combined diff across the stack is byte-identical to the originally-reviewed work.